Block Editor: Update Block Editor classNames to convention #14420

aduth · 2019-03-13T19:27:48Z

Previously: #14112

This nightmare of a pull request seeks to update class names for components moved from the editor package to block-editor, in order to continue respecting the CSS class naming guidelines which prescribe that each component be prefixed by its package directory. It does so without breaking compatibility by keeping the .editor- prefix as a compatibility mechanism.

In its current incarnation, it is as conservative as possible to keeping full compatibility. Unlike my previous comment by which I'd jynxed myself, automated transforms are highly fraught with issues (dealing with classnames, string template interpolation, etc). Conversely, I chose not to get into specifics of what's "worth" keeping for compatibility, for the sake of keeping the effort short-lived and as non-contentious as possible. In both of these instances, I think revisiting in the future may be appropriate.

There is an argument to be made that we make a specific exception to the CSS naming guidelines for the block-editor module, allowing both editor and block-editor components to share the editor- prefix for the sake of avoiding this tedious transition. My cautions against this are that (a) exceptions are bad because they break expectations and (b) it introduces the risk of unintentional conflict when working with components across the two otherwise distinct modules.

Implementation notes:

The changes here were partly RegEx-assisted mass string replacements, with a supplementary (sometimes involved) manual curation.

Specific replacement patterns:

Replace string occurrences, exempting icon slugs:
- Replace: (['"])editor-(?!aligncenter|alignleft|alignright|bold|break|code|contract|customchar|expand|help|indent|insertmore|italic|justify|kitchensink|ltr|ol-rtl|ol|outdent|paragraph|paste-text|paste-word|quote|removeformatting|rtl|spellcheck|strikethrough|table|textcolor|ul|underline|unlink|video)([^ '"]+)
- With: $1editor-$2 block-editor-$2
- Note: In initial passes, this missed some components where the string does not begin with editor- (example)
Remove editor- occurrence for functions expecting a singular argument
- Replace: (hasClass|contains)\( 'editor-.*?
- With: $1( '
Remove editor- occurrence for IDs
- Replace: (id|htmlFor|aria-.+?)(="|': ')editor-.*?
- With: $1$2
- Note: There is no backwards-compatibility for IDs. They are largely used exclusively for the in-app label association.
Replace CSS selector occurrences for deprecated components
- Replace: \.editor-(autocomplete|alignment-toolbar|block-alignment-toolbar|block-controls|block-edit|block-format-controls|block-icon|block-inspector|block-list|block-mover|block-navigation-dropdown|block-selection-clearer|block-settings-menu|block-title|block-toolbar|color-palette|contrast-checker|copy-handler|default-block-appender|inserter|inner-blocks|inspector-advanced-controls|inspector-controls|panel-color-settings|plain-text|rich-text|media-placeholder|media-upload|multi-blocks-switcher|multi-select-scroll-into-view|navigable-toolbar|observe-typing|preserve-scroll-in-reorder|skip-to-selected-block|url-input|url-popover|warning|writing-flow)
- With: .block-editor-$1
- Note: This does not cover all migrated components, only those which had been exported publicly. Additional searches for .editor- were manually performed (guided in some cases by "post" exception \.editor-(?!post-)).

Notable exceptions:

We have some occurrences of clever concatenation (example). Template string interpolation in general proved tricky.
Native components bypassed compatibility and upgraded the class names directly (example)

Testing Instructions:

Give it a thorough shake, paying very close attention to visual regressions.

End-to-end tests should also reveal some issues, if present.

npm run test-e2e

Most useful review will be in considering what may have been overlooked or wrongly revised.

aduth · 2019-03-13T19:51:49Z

It took all of 20 minutes for this to receive its first merge conflict 😩

aduth · 2019-03-13T20:04:42Z

More notes:

Previously mentioned patterns often didn't accommodate for XPath selectors (example)
In general, there's lots of references to editor modules in e2e-tests and e2e-test-utils. Ultimately, I took to just reviewing every occurrence of the plaintext term editor- within the directories.

youknowriad · 2019-03-14T08:44:53Z

Looks like there's a valid e2e test failure but yeah let's not let this PR linger too much, so I'm approving.

aduth · 2019-03-14T13:25:13Z

The current failure is related to a few editor- class names applied outside the context of the editor or block editor modules:

gutenberg/packages/format-library/src/link/inline.js

Line 45 in 589a634

className="editor-format-toolbar__link-container-content"

gutenberg/packages/format-library/src/link/inline.js

Line 62 in 589a634

    
           const linkClassName = classnames( 'editor-format-toolbar__link-container-value', {

gutenberg/packages/format-library/src/link/inline.js

Line 85 in 589a634

className="editor-format-toolbar__link-container-content"

These shouldn't be assigned here, and could probably be abstracted to a component offered by BlockEditor. I won't assume to "correct" it here, for the purpose of keeping this short-lived, but I will update the class names to add the additional block-editor- prefixed class name. This will ensure consistency with what I assume is otherwise meant to extend the <FormatToolbar /> component.

cc @ellatrix

aduth · 2019-03-14T13:36:57Z

The above prompted me to do another search pass for the "editor-" plaintext term across all packages excluding packages/editor, packages/edit-post (I'd also previously done an audit yesterday for e2e-tests and e2e-test-utils).

There are several other erroneous occurrences:

gutenberg/packages/components/src/positioned-at-selection/index.js

Line 60 in 776c0e1

gutenberg/packages/edit-post/src/components/block-settings-menu/plugin-block-settings-menu-group.js

Line 21 in 776c0e1

gutenberg/packages/edit-post/src/components/block-settings-menu/plugin-block-settings-menu-item.js

Line 93 in 776c0e1

className="editor-block-settings-menu__control"

gutenberg/packages/edit-post/src/components/visual-editor/block-inspector-button.js

Line 39 in 776c0e1

className="editor-block-settings-menu__control"

gutenberg/packages/format-library/src/image/index.js

Line 124 in 776c0e1

className="editor-format-toolbar__image-container-content"

gutenberg/packages/format-library/src/image/index.js

Line 147 in 776c0e1

className="editor-format-toolbar__image-container-value"

As seen, they're all related to the format toolbar and block settings menu.

Noted previously, FormatToolbar is a block-editor component, so I think it's reasonable to update all these class names to include the block-editor- prefix.

editor-block-settings-menu, however, as best I can tell never had anything to do with the editor module, but rather plugin integrations in edit-post (source). I expect these were either always wrongly-named or neglected as part of a previous effort to extract components out of editor to edit-post. I will not update them.

aduth · 2019-03-14T18:05:58Z

Another useful pattern for auditing block-editor module using look-behinds, supported in the superior Python-based Sublime Text mass search:

(?<!block-)editor-

aduth

One thing we should consider in a subsequent pull request is performing an audit of excessive code line length, refactoring some of the very-long lines made yet-further-longer as of the changes here.

aduth · 2019-03-14T18:00:15Z

packages/block-editor/src/components/block-compare/index.js

@@ -23,8 +23,8 @@ class BlockCompare extends Component {

 		return difference.map( ( item, pos ) => {
 			const classes = classnames( {
-				'editor-block-compare__added': item.added,
-				'editor-block-compare__removed': item.removed,
+				'editor-block-compare__added block-editor-block-compare__added': item.added,


Of my own curiosity, I did confirm that classnames is capable of handling the assigning of two classes with respect to a single boolean property:

⇒ node > var cn = require( 'classnames' ) undefined > cn( { 'foo bar': true } ) 'foo bar'

aduth · 2019-03-14T18:09:04Z

packages/block-library/src/nextpage/editor.scss

@@ -1,4 +1,4 @@
-.editor-visual-editor__block[data-type="core/nextpage"] {
+.block-editor-block-list__block[data-type="core/nextpage"] {


Noting that this was previously very broken, .editor-visual-editor__block doesn't exist anywhere before or after this set of changes. It should be corrected here now.

aduth added the [Package] Block editor /packages/block-editor label Mar 13, 2019

aduth added this to the 5.3 (Gutenberg) milestone Mar 13, 2019

aduth requested a review from youknowriad March 13, 2019 19:27

aduth requested review from ajitbohra, chrisvanpatten, gziolo, jorgefilipecosta, nerrad, noisysocks, notnownikki, ntwb, Soean and talldan as code owners March 13, 2019 19:27

aduth force-pushed the update/block-editor-classnames branch from 216328f to bdad16a Compare March 13, 2019 20:02

youknowriad approved these changes Mar 14, 2019

View reviewed changes

Block Editor: Update Block Editor classNames to convention

790fa23

aduth force-pushed the update/block-editor-classnames branch from 776c0e1 to 790fa23 Compare March 14, 2019 13:40

aduth commented Mar 14, 2019

View reviewed changes

aduth merged commit 7964f38 into master Mar 14, 2019

aduth deleted the update/block-editor-classnames branch March 14, 2019 18:12

This was referenced Mar 18, 2019

Bump plugin version to 5.3.0-rc.1 #14489

Merged

Update plugin version to 5.3 #14523

Merged

Update plugin version to 5.3 #14524

Closed

Update WordPress trunk to Gutenberg 5.3 #14532

Closed

youknowriad pushed a commit that referenced this pull request Mar 20, 2019

Block Editor: Update Block Editor classNames to convention (#14420)

6def7e4

youknowriad pushed a commit that referenced this pull request Mar 20, 2019

Block Editor: Update Block Editor classNames to convention (#14420)

2a49a46

This was referenced Apr 1, 2019

Edit Post: Update PluginBlockSettingsMenuItem to use block-editor class name #14745

Merged

Fix Blocker Picker autocompleter icon spacing #14772

Merged

This was referenced Apr 10, 2019

Documentation: Add coding guidelines enforcing consistency in class names #8027

Open

Core Columns Block - adds ability to vertically align all the columns #13899

Merged

gchtr mentioned this pull request May 10, 2019

Fix id attribute to match aria-owns attribute #15564

Merged

5 tasks

aduth mentioned this pull request Nov 25, 2019

Add a backward compatibility document #18499

Merged

aduth mentioned this pull request Dec 10, 2019

Block Editor: Remove legacy "editor-" class name compatibility #19050

Merged

This was referenced Jan 17, 2020

Gutenberg 7.3.0: Trigger E2E tests Automattic/wp-calypso#38857

Merged

Gutenberg 7.2.0: replace legacy editor- classes with block-editor- Automattic/jetpack#14446

Merged

ockham mentioned this pull request Feb 6, 2020

EditorRegions: Move to block-editor package, rename to __experimentalEditorSkeleton #20050

Merged

5 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Block Editor: Update Block Editor classNames to convention #14420

Block Editor: Update Block Editor classNames to convention #14420

aduth commented Mar 13, 2019

aduth commented Mar 13, 2019

aduth commented Mar 13, 2019

youknowriad commented Mar 14, 2019

aduth commented Mar 14, 2019

aduth commented Mar 14, 2019

aduth commented Mar 14, 2019

aduth left a comment

aduth Mar 14, 2019

aduth Mar 14, 2019

		@@ -1,4 +1,4 @@
		.editor-visual-editor__block[data-type="core/nextpage"] {
		.block-editor-block-list__block[data-type="core/nextpage"] {

Block Editor: Update Block Editor classNames to convention #14420

Block Editor: Update Block Editor classNames to convention #14420

Conversation

aduth commented Mar 13, 2019

aduth commented Mar 13, 2019

aduth commented Mar 13, 2019

youknowriad commented Mar 14, 2019

aduth commented Mar 14, 2019

aduth commented Mar 14, 2019

aduth commented Mar 14, 2019

aduth left a comment

Choose a reason for hiding this comment

aduth Mar 14, 2019

Choose a reason for hiding this comment

aduth Mar 14, 2019

Choose a reason for hiding this comment